Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1/2] Swift Concurrency migration - Financial Connections API client #4444

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

mats-stripe
Copy link
Collaborator

@mats-stripe mats-stripe commented Jan 8, 2025

Summary

This PR introduces a new Swift Concurrency based API client to the Financial Connections SDK, laying the groundwork for an eventual full migration. The changes are implemented in a way that allows for a gradual rollout without disrupting existing functionality. To do this, we introduce;

  1. FinancialConnectionsAsyncAPIClient: A new API client fully powered by Swift Concurrency.

Some implementation details:

  • The facade implements the FinancialConnectionsAPI protocol, maintaining the current interface for API client consumers throughout the SDK.
  • This approach allows us to isolate changes and continue using the existing API client in production while testing the new async implementation.

This work also provides a shape for the new async API. The current API is shaped as:

// Futures-based API
func synchronize(clientSecret: String, returnURL: String?) -> Future<FinancialConnectionsSynchronize>

While the new API is shaped as:

// Async-based API
func synchronize(clientSecret: String, returnURL: String?) async throws -> FinancialConnectionsSynchronize

A toggle has been added to the FC example app to switch between API client implementations. This is currently the only way to test the async API client:

image

Motivation

Move towards migration to Swift Concurrency

Next Steps: Part 2 will focus on migrating existing consumers to use the new async API client.

Testing

Unit tests added, and manual testing done.

Screen.Recording.2025-01-08.at.1.47.18.PM.mov

Changelog

N/a

Copy link

emerge-tools bot commented Jan 8, 2025

2 builds increased size, 6 builds had no size change

Name Version Download Change Install Change Approval
StripeSize
com.stripe.StripeSize
1.0 (1) 2.1 MB ⬆️ 3 B 6.9 MB - N/A
StripePaymentsSize
com.stripe.StripePaymentsSize
1.0 (1) 1.2 MB - 4.2 MB - N/A
StripePaymentsUISize
com.stripe.StripePaymentsUISize
1.0 (1) 1.9 MB ⬇️ 2 B 6.4 MB - N/A
StripePaymentSheetSize
com.stripe.StripePaymentSheetSize
1.0 (1) 3.7 MB ⬆️ 6 B 11.1 MB - N/A
StripeIdentitySize
com.stripe.StripeIdentitySize
1.0 (1) 1.4 MB ⬆️ 1 B 4.3 MB - N/A
StripeApplePaySize
com.stripe.StripeApplePaySize
1.0 (1) 484.8 kB - 1.7 MB - N/A
⚠️ StripeFinancialConnectionsSize
com.stripe.StripeFinancialConnectionsSize
1.0 (1) 1.5 MB ⬆️ 66.3 kB (4.75%) 4.7 MB ⬆️ 191.4 kB (4.28%) N/A
⚠️ StripeConnectSize
com.stripe.StripeConnectSize
1.0 (1) 1.6 MB ⬆️ 67.6 kB (4.34%) 5.2 MB ⬆️ 193.3 kB (3.88%) N/A

StripeSize 1.0 (1)
com.stripe.StripeSize

No changes to report

StripePaymentsSize 1.0 (1)
com.stripe.StripePaymentsSize

No changes to report

StripePaymentsUISize 1.0 (1)
com.stripe.StripePaymentsUISize

No changes to report

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

No changes to report

StripeIdentitySize 1.0 (1)
com.stripe.StripeIdentitySize

No changes to report

StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize

No changes to report

StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 191.4 kB (4.28%)
Total download size change: ⬆️ 66.3 kB (4.75%)

Largest size changes

Item Install Size Change
Swift._NativeDictionary.merge(isUnique,uniquingKeysWith) ⬆️ 4.4 kB
Code Signature ⬆️ 4.2 kB
🗑 StripeFinancialConnections.NativeFlowAPIDataManager.manifest ⬇️ -2.0 kB
StripeFinancialConnections.NativeFlowAPIDataManager.init(manifest... ⬇️ -1.8 kB
StripeFinancialConnectionsSize ⬆️ 1.3 kB
View Treemap

Image of diff

StripeConnectSize 1.0 (1)
com.stripe.StripeConnectSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 193.3 kB (3.88%)
Total download size change: ⬆️ 67.6 kB (4.34%)

Largest size changes

Item Install Size Change
Code Signature ⬆️ 5.0 kB
Swift._NativeDictionary.merge(isUnique,uniquingKeysWith) ⬆️ 4.4 kB
🗑 StripeFinancialConnections.NativeFlowAPIDataManager.manifest ⬇️ -2.0 kB
StripeFinancialConnections.NativeFlowAPIDataManager.init(manifest... ⬇️ -1.8 kB
StripeConnectSize ⬆️ 1.3 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

Copy link

github-actions bot commented Jan 8, 2025

🚨 New dead code detected in this PR:

FinancialConnectionsAsyncAPIClient.swift:169 warning: Protocol 'FinancialConnectionsAsyncAPI' is redundant as it's never used as an existential type
FinancialConnectionsAsyncAPIClient.swift:348 warning: Protocol 'FinancialConnectionsAsyncAPI' conformance is redundant

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

@mats-stripe mats-stripe changed the title Mats/async fc api client Create new FC API client using Swift Concurrency Jan 8, 2025
@mats-stripe mats-stripe changed the title Create new FC API client using Swift Concurrency [1/2] Swift Concurrency migration - Financial Connections API client Jan 8, 2025
@mats-stripe mats-stripe force-pushed the mats/async_fc_api_client branch from 0da754c to 70bd939 Compare January 9, 2025 16:17
@mats-stripe mats-stripe marked this pull request as ready for review January 9, 2025 16:43
@mats-stripe mats-stripe requested review from a team as code owners January 9, 2025 16:43
@mats-stripe mats-stripe force-pushed the mats/async_fc_api_client branch from 323a063 to 0e1c5f1 Compare January 14, 2025 20:07
@@ -426,6 +441,11 @@ private func PresentFinancialConnectionsSheet(
)
financialConnectionsSheet.apiClient.stripeAccount = stripeAccount
financialConnectionsSheet.onEvent = onEvent

let elementsSessionContext = StripeCore.ElementsSessionContext(
usesExperimentalAsyncAPIClient: useAsyncApiClient
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure that we want to have this in ElementsSessionContext. Can we stuff it in UserDefaults while we’re testing everything? That makes it easy to rip out again once we fully migrate over.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do that!

@mats-stripe mats-stripe force-pushed the mats/async_fc_api_client branch from 0e1c5f1 to 8b24b2d Compare January 16, 2025 16:55
@mats-stripe mats-stripe force-pushed the mats/async_fc_api_client branch from 8b24b2d to b81d579 Compare January 16, 2025 16:59
@@ -301,6 +303,12 @@ protocol FinancialConnectionsAPI {
paymentDetailsId: String,
billingDetails: ElementsSessionContext.BillingDetails?
) -> Future<LinkBankPaymentMethod>

func updateAvailableIncentives(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@mats-stripe mats-stripe merged commit 93ee7c7 into master Jan 16, 2025
6 checks passed
@mats-stripe mats-stripe deleted the mats/async_fc_api_client branch January 16, 2025 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants